20. Valid Parentheses（251019) #6
Conversation
Walkthrough
Changes
Sequence Diagram(s)sequenceDiagram
participant Input as 入力文字列
participant Validator as isValid
participant Stack as スタック
Input->>Validator: 文字列を送る
Note over Validator: 1文字ずつ処理
alt 開き括弧
Validator->>Stack: push(開き)
Note right of Stack: 対応する閉じを期待リストへ追加（或いはスタックに保持）
else 閉じ括弧
Validator->>Stack: peek/pop で比較
alt 一致
Stack->>Validator: 継続
else 不一致または空
Validator->>Validator: false を返す
end
end
Note over Validator: 全処理後
alt スタック空
Validator->>Validator: true を返す
else
Validator->>Validator: false を返す
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–25 分
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
20.ValidParentheses_251019.md (1)
34-36: open_to_closeマッピングの選択は適切です
open_to_closeマッピングが推奨される理由：開き括弧を見つけたときにスタックに積み、後で対応する閉じ括弧を検証するというアルゴリズムの流れに自然に合致します。close_to_openの場合、閉じ括弧を見つけたときにスタックトップをルックアップする必要があり、直感的ではなく、逆引きの複雑さが増します。現在のアプローチは標準的で理解しやすいものです。3～4ヶ月後に独立して異なるアプローチを思いついたことは、問題理解が深まった証です。LeetCodeの模範答案を確認する前に、このアプローチを完成させることは良い学習戦略です。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
20.ValidParentheses_251019.md(1 hunks)
🔇 Additional comments (1)
20.ValidParentheses_251019.md (1)
3-32: 初回の試行が学習プロセスの価値を示している初回の解答は完成していませんが、括弧マッチングの問題本質を理解し、辞書ベースのアプローチを検討している様子が伝わります。スタック手法への発展につながる有益な思考過程です。
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
20.ValidParentheses_251019.md (1)
95-95: 参考リンクをMarkdownリンク形式で記述してください。Static analysis hint: MD034 (bare URL detected)
行95の参考リンクを括弧で括り、リンク形式に統一します：
- - https://github.com/h1rosaka/arai60/pull/8/files + - [参考: プッシュダウンオートマトン](https://github.com/h1rosaka/arai60/pull/8/files)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
20.ValidParentheses_251019.md(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
20.ValidParentheses_251019.md
95-95: Bare URL used
(MD034, no-bare-urls)
🔇 Additional comments (2)
20.ValidParentheses_251019.md (2)
74-91: 修正版のスタック実装は正確で適切です。括弧の優先度を明示する括弧を追加することで、前のバージョンの問題を解決しました（89行目）。アルゴリズムは以下の点で正確です：
- 開き括弧をスタックに積み、対応する閉じ括弧の期待値を構築
s == correct_sで入力文字列と一致確認len(open_brackets_stack) == 0で未クローズ括弧がないか確認括弧のネスト、不一致、未クローズのケースをすべて正しく処理します。
60-71: 問題の自己診断と修正の説明が優れています。Pythonの三項演算子の優先度を正確に分析し、問題を特定し、修正理由を明確に記録しました。このような問題を発見して対応できる能力は重要です。
| correct_s = correct_s + char | ||
| else: | ||
| correct_s = correct_s + open_to_close[open_brackets_stack.pop()] if open_brackets_stack else "#" | ||
| return s == correct_s and len(open_brackets_stack) == 0 |
There was a problem hiding this comment.
まあ、いいんですが、s == correct_s にならない条件ってなんですかね。
open_to_close[open_brackets_stack.pop()] のあたりでもう分かりませんか。
There was a problem hiding this comment.
すみません、意図が分からずです。s==correct_sを行うアプローチだと書き方が複雑なので、悪手ということでしょうか。
s==correct_sにならないような条件は端的には求められず、以下のように複数の条件分岐を使用して検出しました。
(別解:不正な値を検出して検出されなかったパターンを正とする方式)
class Solution(object):
def isValid(self, s):
"""
:type s: str
:rtype: bool
"""
open_brackets_stack = []
open_to_close = {'(': ')', '{': '}', '[' : ']'}
for char in s:
if char in open_to_close:#charが開括弧の場合
close_brackets = open_to_close[char]
open_brackets_stack.append(close_brackets)
else: #charが閉括弧の場合
if len(open_brackets_stack) != 0: # 開括弧と閉括弧の混在している場合
if open_brackets_stack.pop() != char: #不正ペアの検知
return False
if len(s) % 2 == 1: #ペアを数的に組めない余りがいるパターン”例：({}”を除外
return False
else: # 閉じ括弧のみの場合
return False
if len(open_brackets_stack) == len(s): return False #開括弧だけの場合を除外
if len(open_brackets_stack) != 0: return False #開括弧と閉括弧の混在していて、開括弧と閉括弧の数が等しくない場合"[]]]"を除外
return True
そこから以下のように捉えました。
別解の方がパフォーマンスが良かった。3ms。これまでの解答は10ms。
旧解答は最後まで処理をしないと判断できないからパフォーマンスが悪いのかもしれません。
There was a problem hiding this comment.
より早い段階で False になることが判別できるということだと思います。より具体的には、open_to_close[open_brackets_stack.pop()] != char で、 s != correct_s が確定すると思います。
There was a problem hiding this comment.
上記のソースコードは、よりシンプルにできると思います。
class Solution(object):
def isValid(self, s):
open_brackets = []
open_to_close = {'(': ')', '{': '}', '[' : ']'}
for char in s:
if char in open_to_close:
# charが開括弧の場合
open_brackets.append(char)
else:
# charが閉括弧の場合
if not open_brackets:
return False
if open_to_close[open_brackets.pop()] != char:
return False
return not open_brackets
| open_brackets_stack.append(char) | ||
| correct_s = correct_s + char | ||
| else: | ||
| correct_s = correct_s + open_to_close[open_brackets_stack.pop()] if open_brackets_stack else "#" |
There was a problem hiding this comment.
if 文と参考演算子が混在しているせいで読みにくく感じました。また、 1 行が長すぎるように感じました。 if 文で書き下したほうが読みやすくなると思います。
| correct_s = correct_s + char | ||
| else: | ||
| correct_s = correct_s + open_to_close[open_brackets_stack.pop()] if open_brackets_stack else "#" | ||
| return s == correct_s and len(open_brackets_stack) == 0 |
There was a problem hiding this comment.
Implicit false を利用し、
return s == correct_s and not open_brackets_stackと書くことをおすすめします。
参考までにスタイルガイドへのリンクを貼ります。
https://google.github.io/styleguide/pyguide.html#2144-decision
For sequences (strings, lists, tuples), use the fact that empty sequences are false, so if seq: and if not seq: are preferable to if len(seq): and if not len(seq): respectively.
上記のスタイルガイドは唯一絶対のルールではなく、複数あるスタイルガイドの一つに過ぎないということを念頭に置くことをお勧めします。また、所属するチームにより何が良いとされているかは変わります。自分の中で良い書き方の基準を持ちつつ、チームの平均的な書き方で書くことをお勧めいたします。
| if char in open_to_close: | ||
| open_brackets_stack.append(char) | ||
| correct_s = correct_s + char | ||
| else: | ||
| correct_s = correct_s + (open_to_close[open_brackets_stack.pop()] if open_brackets_stack else "#" ) |
There was a problem hiding this comment.
correct_s = correct_s + char ですが、forループを書き下すと '' + 's[0]' + 's[1]' + ... + 's[-1]' のようなイミュータブルなシーケンスの結合になっており、パフォーマンスの観点から避けるべきだと思います。
イミュータブルなシーケンスの結合は、常に新しいオブジェクトを返します。これは、結合の繰り返しでシーケンスを構築する実行時間コストがシーケンスの長さの合計の二次式になることを意味します。実行時間コストを線形にするには、代わりに以下のいずれかにしてください:
- str オブジェクトを結合するには、リストを構築して最後に str.join() を使うか、 io.StringIO インスタンスに書き込んで完成してから値を取得してください
...
https://docs.python.org/ja/3.13/library/stdtypes.html#string-methods
| correct_s = correct_s + char | ||
| else: | ||
| correct_s = correct_s + (open_to_close[open_brackets_stack.pop()] if open_brackets_stack else "#" ) | ||
| return s == correct_s and len(open_brackets_stack) == 0 |
There was a problem hiding this comment.
correct_sという変数を設けずに実装する方がシンプルに感じます。
具体的にはopen_to_close[open_brackets_stack.pop()]とcharを比べることで、適切な閉じカッコかを確認します。そうすればループ中に不正な文字が来た時点で処理を打ち切ることができ、最後にはlen(open_brackets_stack) == 0の比較だけで済みます。
|
|
||
| (2回目の回答の修正) | ||
| ```python | ||
| class Solution(object): |
There was a problem hiding this comment.
Python3では、class Solution: で大丈夫です。
もしかすると、leetcodeの設定がpython3になっていないのかもしれません。
pythonとpython3の２つが選べるようになっており紛らわしいのですが、昨今はpython3が主流と思われるので、特別な理由がなければpython3を選んでおいたほうが良さそうです。
|
レビューありがとうございます。レスが遅くなり申し訳ありません。 |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
20.ValidParentheses_251019.md (1)
102-120: Line 118: 演算子優先度の問題が繰り返されている118行目の三項演算子は過去のレビューで指摘されたのと同じ優先度の問題を抱えています。Pythonの三項演算子は
value_if_true if condition else value_if_falseで評価されるため、現在のコードは以下のように動作します：# 現在のコード (問題あり) correct_s = correct_s + open_brackets_stack.pop() if open_brackets_stack else "#" # 解析結果: correct_s = (correct_s + open_brackets_stack.pop()) if open_brackets_stack else "#" # スタックが空の場合: correct_s = "#" (置き換え、追記ではない)括弧で優先度を明示してください：
- correct_s = correct_s + open_brackets_stack.pop() if open_brackets_stack else "#" + correct_s = correct_s + (open_brackets_stack.pop() if open_brackets_stack else "#")また、以下の改善も推奨します：
文字列連結のパフォーマンス: ループ内での
correct_s + charは O(n²) の時間計算量になります。過去のレビューでも指摘されているとおり、リストを使って最後にjoin()する方が効率的です。Pythonの慣例に従う:
len(open_brackets_stack) == 0の代わりにnot open_brackets_stackを使う方が、Pythonのスタイルガイドに沿っています（Google Python Style Guide 参照）。
🧹 Nitpick comments (3)
20.ValidParentheses_251019.md (3)
124-147: 別解のアプローチを推奨する代替案（124-147行目）はメイン解答（102-120行目）より約3倍高速（3ms vs 10ms）です。早期リターン戦略により、不正な組み合わせを検出した時点で即座に処理を終了できるため、文字列全体を処理する必要がありません。
LeetCodeでの検証問題では、このような早期リターン戦略が望ましいアプローチです。メイン解答の
correct_s == sの比較方式は複雑で、パフォーマンスにも劣ります。別解をメイン解答として採用することを検討してください。
95-95: Markdownの形式を修正：URLをリンク化する95行目の裸のURLはMarkdown形式で適切にリンク化してください：
-- https://github.com/h1rosaka/arai60/pull/8/files +- [参考: プッシュダウンオートマトン](https://github.com/h1rosaka/arai60/pull/8/files)
149-149: コードフェンスに言語指定を追加する149行目のコードフェンスに言語指定がありません。以下のように修正してください：
-``` +``` 別解の方がパフォーマンスが良かった。3ms。これまでの解答は10ms。または、このセクションがテキストのみであれば、コードフェンスで囲む必要はありません。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
20.ValidParentheses_251019.md(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
20.ValidParentheses_251019.md
95-95: Bare URL used
(MD034, no-bare-urls)
149-149: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
20.ValidParentheses_251019.md (1)
118-118: 三項演算子の優先度問題を修正してください（前回と同じ指摘です）このコードは以前のレビューで指摘された演算子優先度の問題と同じです。スタックが空のときに
correct_sが"#"に置き換わってしまいます。前回の修正（89行目）のように括弧を追加してください：
-correct_s = correct_s + open_brackets_stack.pop() if open_brackets_stack else "#" +correct_s = correct_s + (open_brackets_stack.pop() if open_brackets_stack else "#")
🧹 Nitpick comments (1)
20.ValidParentheses_251019.md (1)
126-150: 別解のアプローチが優れています126-150行目の別解は、不正な括弧を早期に検出することで、パフォーマンスが大幅に改善されています（10ms → 3ms）。このアプローチは以下の利点があります：
- 最後まで処理を続ける必要がない
- 早期終了により無駄な計算が削減される
correct_s文字列の構築が不要ただし
correct_sを使わずに直接比較する方式の方が、デバッグと保守がしやすい可能性があります。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
20.ValidParentheses_251019.md(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
20.ValidParentheses_251019.md
95-95: Bare URL used
(MD034, no-bare-urls)
152-152: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
| if open_brackets_stack: | ||
| correct_s = correct_s + open_brackets_stack.pop() | ||
| else: "#" |
There was a problem hiding this comment.
行176の不完全なコードを修正してください
else 分岐内の "#" は評価されるだけで何も行いません（死コード）。意図の確認が必要です：
- もし
correct_sに"#"を追加したい場合は：correct_s = correct_s + "#" - もし何もしない場合は、その行は削除できます
推奨の修正：
else:
if open_brackets_stack:
correct_s = correct_s + open_brackets_stack.pop()
- else: "#"
+ else:
+ correct_s = correct_s + "#"Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In 20.ValidParentheses_251019.md around lines 174–176, the else branch contains
a bare "#" expression that does nothing; replace it with the intended behavior:
if you meant to append a "#" to correct_s, change the line to assign correct_s =
correct_s + "#" (or use += "#"), otherwise remove the entire else branch so
there is no dead code.
…および文字列イミュータブル性の注記を追記
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
20.ValidParentheses_251019.md (1)
175-179: 行178: 死コード — 単独の"#"は何もしません行178の
else: "#"は文字列リテラルを評価するだけで、何も実行しません（死コード）。意図を確認してください：
correct_sに"#"を追加したい場合：correct_s = correct_s + "#"- 何もしない場合：このブロックを削除
現在のコードでは、
correct_sは更新されないため、ロジックが破綻しています。else: if open_brackets_stack: correct_s = correct_s + open_brackets_stack.pop() - else: "#" + else: + correct_s = correct_s + "#"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
20.ValidParentheses_251019.md(1 hunks)
🔇 Additional comments (2)
20.ValidParentheses_251019.md (2)
188-206: 推奨解答：行188-206の実装が最適ですこの版は以下の理由で最も推奨されます：
- 効率性：文字列の再構築がなく、スタック操作のみで O(n) 時間計算量
- 可読性：if/else で明確に制御フロー
- 正確性：早期リターンでエラーを即座に検出
- Pythonic：
not open_bracketsで暗黙の False を活用（Googleスタイルガイド準拠）この実装をデフォルトの推奨解答として採用することをお勧めします。
208-219: 優れた説明 — 不変条件に基づいた解説は非常に参考になりますループ内で「これまで読んだ部分が正しい括弧列の途中状態であることを保つ」という不変条件の説明は、アルゴリズムの本質を的確に説明しており、学習教材として優秀です。
このような解説があることで、コードの背後にある設計思想が明確になります。
| for char in s: | ||
| if char in open_to_close: | ||
| open_brackets_stack.append(char) | ||
| correct_s = correct_s + char |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
文字列連結のパフォーマンス — ループ内での繰り返し連結を避けてください
複数の解答で文字列の繰り返し連結が使われています（行53, 87, 116-117, 174-177）。Python の文字列はイミュータブルなため、ループ内での繰り返し連結は O(n²) の計算量になります。
ドキュメント内で「旧解答は10ms」と記載されているのは、この連結のコストが大きな理由の一つです。
改善方法：
- リストを使い、最後に
''.join()する - または、そもそも
correct_s変数を使わない（行188-206の最終版がこれ）
行188-206の解答が最も推奨されます。この版は文字列を再構築せず、スタックから直接比較するため、O(n) で効率的です。
# ✅ 推奨：文字列再構築なし
for char in s:
if char in open_to_close:
open_brackets.append(char)
else:
if not open_brackets:
return False
if open_to_close[open_brackets.pop()] != char:
return False
return not open_bracketsAlso applies to: 87-87, 116-117, 174-177
🤖 Prompt for AI Agents
In 20.ValidParentheses_251019.md around line 53 (and also at lines ~87, 116-117,
174-177), the code repeatedly concatenates to a string inside loops (e.g.,
correct_s = correct_s + char), causing O(n²) behavior; replace these in-loop
concatenations with appending to a list and doing ''.join(list) after the loop
or—preferably—refactor to avoid reconstructing strings entirely by using the
stack-based approach shown at lines 188-206 (push opens to a list and
compare/pop against closes), which yields O(n) time and removes the need for
correct_s.
| else: | ||
| correct_s = correct_s + open_brackets_stack.pop() if open_brackets_stack else "#" | ||
| return s == correct_s and len(open_brackets_stack) == 0 |
There was a problem hiding this comment.
行118の三項演算子の優先度に注意
行55の同じパターンです。現在のコードでは、スタックが空の場合に correct_s は "#" に置き換わり、追記されません。
# 現在のコード (問題あり)
correct_s = correct_s + open_brackets_stack.pop() if open_brackets_stack else "#"
# 評価結果:
correct_s = (correct_s + open_brackets_stack.pop()) if open_brackets_stack else "#"括弧で優先度を明示してください：
- correct_s = correct_s + open_brackets_stack.pop() if open_brackets_stack else "#"
+ correct_s = correct_s + (open_brackets_stack.pop() if open_brackets_stack else "#")🤖 Prompt for AI Agents
In 20.ValidParentheses_251019.md around lines 117–119, the ternary expression is
binding to the assignment instead of the concatenation, causing correct_s to
become just "#" when the stack is empty; fix by parenthesizing the conditional
so the concatenation uses the conditional result (e.g., correct_s = correct_s +
(conditional-expression)), or alternatively use += with the conditional
expression or an explicit if/else to append "#" only when the stack is empty.
| if len(open_brackets_stack) != 0: # 開括弧と閉括弧の混在している場合 | ||
| if open_brackets_stack.pop() != char: #不正ペアの検知 | ||
| return False | ||
| if len(s) % 2 == 1: #ペアを数的に組めない余りがいるパターン”例：({}”を除外 | ||
| return False | ||
| else: # 閉じ括弧のみの場合 | ||
| return False |
There was a problem hiding this comment.
行143の len(s) % 2 == 1 チェックの位置と論理に問題あり
このチェックはループ内部、閉じ括弧のポップ後に毎回実行されていますが、文字列全体の長さが奇数かどうかはループ内のどこで判定しても同じ結果です。また、この場所でのチェックは意図が不明確です。
正しくは、開括弧と閉括弧の数が一致しているかは、ループ終了後に len(open_brackets_stack) == 0 で確認すれば十分です。
ループ内でのチェックを削除することをお勧めします：
if open_brackets_stack.pop() != char: #不正ペアの検知
return False
- if len(s) % 2 == 1: #ペアを数的に組めない余りがいるパターン"例：({}"を除外
- return False🤖 Prompt for AI Agents
In 20.ValidParentheses_251019.md around lines 140 to 146, remove the in-loop
check "if len(s) % 2 == 1" (do not perform a global-length parity check inside
the per-character closing-bracket handling); instead rely on stack consistency —
after the main loop completes, return True only if len(open_brackets_stack) == 0
and otherwise return False (i.e., delete the parity check inside the loop and
add a single post-loop stack-empty check).
問題リンク：https://leetcode.com/problems/valid-parentheses/description/
3、4ヶ月ぶりに解いたところ、初回の回答(leetcodeの模範回答)とは別の方法で解いてしまったため、改めてレビューをお願いしたいです。
Summary by CodeRabbit